Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: create tests for grid equality #183

Merged
merged 2 commits into from
May 19, 2020
Merged

test: create tests for grid equality #183

merged 2 commits into from
May 19, 2020

Conversation

danielolsen
Copy link
Contributor

Purpose

Add tests for the grid equality feature (#152)

What is the code doing

Testing some modifications which should cause grid equality to return False, as well as some which should cause it to return True.

Time to review

5-15 minutes.

Copy link
Collaborator

@rouille rouille left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the tests pass. I also went through the tests and it seems that most cases are covered (dropped columns, gencost, storage, ..). Thanks. One comment though, we might want in the future to define one Grid object outside of the test and use it for all the tests in order to speed up things.

@danielolsen
Copy link
Contributor Author

@rouille agreed, this could be much more time-efficient by combining several of these tests into one function (only defining ref_grid once, copying as necessary, with several individual asserts), or in a unittest.TestCase class with a setUp function. I PR'ed more of an MVP so that this branch could be used right away to aid in #182. I'll clean it up further before merging.

@danielolsen
Copy link
Contributor Author

I refactored the changes to use one copy of the Texas grid and one copy of the Western grid, via @pytest.fixture(), calling copy.deepcopy as necessary. This cut the total test runtime in half, from ~35 seconds to ~17 seconds. For reference, the tests currently in develop take ~14 seconds, so the pytest fixture cut down the added time by a factor of ~6.

@danielolsen danielolsen merged commit 0ffeb47 into develop May 19, 2020
@danielolsen danielolsen deleted the grid_eq_tests branch May 19, 2020 00:21
@danielolsen
Copy link
Contributor Author

For posterity's sake: building these tests using the unittest.TestCase framework, with a setUp method that pre-built a Western grid and a Texas grid, was no faster to run in pytest than the original version where tests shared no data. I suspect that pytest was running the setUp before each test, rather than running it just once, hence the use of @pytest.fixture().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants